fix(golang): a call whose context ends fails on its own, and leaves the client open by coderdan · Pull Request #1263 · cipherstash/stack

coderdan · 2026-10-18T10:36:35Z

Summary

The Go SDK shares one encrypt.Client across all of a process's requests, and one auth profile behind it. Today, when any single caller cancels a call or hits its deadline, the whole client closes. Every later call then fails with ErrState until the process restarts. This PR makes a cancelled or timed-out call fail on its own and leaves the client, and the auth profile, open for everyone else.

Why it happened

Both wazero runtimes close the module when a call's context ends. They are built with WithCloseOnContextDone, which runs the WebAssembly guest, the compiled Rust engine. Every guest call ran under the caller's own context. So a caller that gave up closed the module that every caller shares.
The client's lock ignored deadlines. It was a plain sync.Mutex. A request whose deadline passed while it waited for the lock still reached wazero, with a context that had already ended. wazero closed the module before any guest code ran.
The token fetch closed the auth profile. The encrypt client fetched its token with Strategy.Token under that same caller context, so a cancelled token fetch closed the auth profile as well.

Changes

Shared plumbing: internal/guest/lock.go adds three pieces that both packages use:
- Lock: a call lock whose wait ends with the caller's context. A context that has already ended never reaches the guest.
- Detach: the guest runs under a context the caller cannot end.
- Caller: host functions (the ZeroKMS request, the token fetch, the auth CTS request and the OIDC provider call) still honour the caller's context. A cancelled request fails as a transport error, and the guest unwinds through its normal error path.
Backstop: if a guest is still running guest.StuckAfter (30 seconds) after its caller gave up, it is treated as stuck and closed, as before. This only happens if there is a bug.
Errors: a cancelled call returns an error that matches both the context's error and the underlying one, for example ErrTransport.
Packages: encrypt.Client, auth.ProfileStore and auth.Strategy use the new lock. Client.call's closure now receives the call's context.
Docs: a Cancellation section in encrypt/doc.go, matching text in auth/doc.go and the README, and a CHANGELOG entry.
A device-session refresh finishes after its caller gives up (second commit). The token service rotates the refresh token when it answers. If the request were cancelled after the service answered, the new token would be lost, and the next refresh would fail with invalid_grant. So Strategy.Token returns to a caller whose context has ended straight away. The refresh carries on in the background for up to 10 seconds, and the guest saves what comes back for the next call. This matches what stack-auth's native refresh does (fix(auth): preserve token refreshes across caller cancellation #1172).
- Only the refresh gets this. Access-key and OIDC exchanges, and ZeroKMS requests, are still cancelled with their caller.
- Strategy.Close: it waits for a refresh already in flight, so the rotated token is kept.
- No token string is made for a caller that has left.

Verification

Refresh tests: TestARefreshAnsweredAfterItsCallerGaveUpIsKept uses a device-session refresh against a token service that retires the old refresh token. Without the grace, it fails with invalid_grant. TestAnAccessKeyExchangeEndsWithItsCaller checks that Close returns promptly after a cancelled exchange. With the grace applied to every request, Close took 10 seconds.
New tests:
- encrypt/cancel_test.go covers a call cancelled during a ZeroKMS request, during a token fetch, and during a real access-key exchange. It also covers a call whose deadline ends in the queue, a stuck guest, and a soak of 16 goroutines × 40 calls with random deadlines, which checks that no goroutines are leaked.
- auth/cancel_test.go covers the auth side.
- internal/guest/lock_test.go has the unit tests.
The tests fail before the fix: on a clean next with the same guests, equivalent tests reproduced all three failures. A cancelled call closed the client. A call whose deadline passed in the queue closed the client: it waited 202 ms instead of leaving at its 20 ms deadline. A cancelled token fetch closed the profile.
Passing locally: go test ./... -count=1, go vet ./... and gofmt, plus go test -race for internal/guest and auth.
-race: the full go test -race ./... passes locally, and a later comment records the run. The cancellation tests also pass under -race after the review fixes. Earlier note: for the encrypt package, only the cancellation and transport tests ran under -race. The full-package race run was stopped after about 20 minutes without finishing.

Review notes

Where to start: internal/guest/lock.go.
Backstop: it is "30 seconds after the caller gave up", not a limit on how long the guest may run. A time limit would close the client during a large batch running under a context with no deadline. It is a constant, not a public option.
Hung host calls: a host function that ignores cancellation, such as a RoundTripper that never returns, still holds the lock until it returns, as before.
Not done: the live soak test in release-golang.yml from the issue's step 7.

changeset-bot · 2026-10-18T10:36:39Z

⚠️ No Changeset found

Latest commit: 94f995d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderabbitai · 2026-10-18T10:36:42Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: b0c20c34-432f-4835-ab13-3f25eadd2e45

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

coderdan · 2026-10-18T10:59:40Z

Ran the full go test -race -count=1 ./... locally on d431e3aab: every package passes with no data races. encrypt took about 9 minutes; auth took 80 seconds.

coderdan

Reviewed both commits against a fresh worktree of d431e3a, with all four guests built (wasm:guest:build, :deterministic, :eql, wasm:auth-guest:build). Ran go vet, gofmt -l (both clean) and the new tests under -race in internal/guest, encrypt and auth: all pass, no races.

What I checked and found sound:

wazero semantics, against the v1.12.0 source. callWithStack only closes the module up front when the call context is already done, and otherwise watches it from CloseModuleOnCanceledOrTimeout; FailIfClosed closes the deferred resources and returns a sys.ExitError whose Is matches context.Canceled. So Detach's context does keep the caller's cancellation away from the module, the 30 s backstop closes it the way the old code did, and a host function that is mid-flight when the backstop fires fails its se_alloc and the guest exits at its next check. Host functions receive the detached context, and Caller recovers the caller's, so the ZeroKMS request, token_get and the OIDC provider call are cancelled with the caller as described.
Detach lifecycle. The AfterFunc is stopped and the timer stopped under the mutex, cancel() always runs from done, and AfterFunc on a context with no Done registers nothing. No timer or goroutine outlives the call; the soak test's goroutine count confirms it.
Lock. Refuses an ended context even when free, re-checks after the select, and a blocked channel send is FIFO, so Close's uninterruptible Lock cannot be starved.
Locks and deadlocks. Order is strategy mu then profile mu, consistently (Token goroutine, Strategy.Close); ProfileStore.Close takes only the profile's. withRefreshLock releases the flock on every return, and a callArgs behind it with an ended context is refused before the guest runs, so the lock is never abandoned and a refresh is never half-done. A device-session refresh whose caller leaves mid-request keeps the flock until the guest has saved auth.json, which is the native crate's behaviour too.
Races. tokenErr and authHTTPStatus.code are written by host functions on the call's goroutine (now the Token goroutine for auth) and read on that same goroutine after the guest returns; the result crosses to the caller through the channel.
Error shape. %w: %w keeps errors.Is for the context error, ErrTransport and ErrState, and errors.As for *Diagnostic. ErrState is still reserved for a closed client.
Docs. The Cancellation section, README, auth/doc.go and the CHANGELOG match the code, including the 10 s grace and the 30 s backstop.

Findings, by severity:

Medium — Strategy.Close (and so a cancelled NewClient) waits out the 10 s grace. Details inline on auth/strategy.go. Verified with a probe: after a Token that returned at its 50 ms deadline, strategy.Close() returned only when the grace expired.
Low — the grace covers every token-service request, not just the refresh that rotates a token, and its cost is paid by every other caller of the profile. Inline on auth/transport.go.
Low — TestACallCancelledDuringATokenExchangeLeavesTheProfileOpen takes ~16 s under -race because it waits for the production grace. Inline on encrypt/cancel_test.go.
Low — two timing assertions have ~100 ms of slack, which is where CI flakes usually come from. Inline on both cancel_test.go files.
Nit — the token a cancelled caller abandons sits unwiped in the result channel. Inline on auth/strategy.go.

None of these contradict the design; 1 is the only one I would fix before merging, and it is small.

coderdan · 2026-10-19T01:00:57Z

Force-pushed: a test-only fix folded into 861df3f. TestAnAccessKeyExchangeEndsWithItsCaller failed intermittently, more often under -race. The client did cancel the held request: a diagnostic showed its round trip ending with the caller's deadline every time. The race was in the test: Go's test server notices a hung-up client a moment after the client leaves, and the test's end could arrive first. The handler's select picked the end, and the test reported a request that was never cancelled. The test now waits at its end, for up to 5 seconds, for each held request to see its cancellation. It waits before the server is closed, because closing the server would end the request itself and hide a real failure. 300 runs of both cancellation tests under -race passed with no failures; before the fix, the same run failed several times.

chatgpt-codex-connector · 2026-10-19T02:59:54Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-19T03:04:49.507472Z	`861df3f`	Draft marked ready
🔒 Security Review	✅ Completed 2026-10-19T03:05:11.925499Z	`861df3f`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

cipherstash-agent

Review attestation: automated `/code-review medium` + `/cross-check-review-findings`

Reviewed head 861df3f.

Result: no critical or blocking findings. No inline comments.

Read in full:

internal/guest/lock.go
the encrypt/client.go, encrypt/guest.go and encrypt/transport.go diffs
the auth/store.go, auth/strategy.go and auth/transport.go diffs

Checked and found sound:

Lock.Acquire refuses an already-ended context, including the case where select picks the lock channel when both cases are ready. It releases the lock before returning the error.
Detach and Caller:
- The guest runs under a context the caller cannot cancel.
- The backstop timer starts only after the caller's context ends, and done() stops it.
- The nested Detach in requestContext keeps the original caller reachable through Caller.
Strategy.Token:
- The goroutine owns the lock and unlocks it on every path.
- No token string is built for a caller that has left, because ctx.Err() is checked before string(out).
- Close waits for the in-flight refresh, which is documented.
The refresh grace (10s) is shorter than StuckAfter (30s), so a refresh that overruns fails as a transport error and the profile stays open.
Context-error wrapping uses %w: %w, so errors.Is still matches ErrAuthRefreshRequired, ErrTransport and the context error.

Not covered:

I did not run the tests, including -race.
I skimmed the new test files, docs and CHANGELOG rather than reviewing them line by line.

Per AGENTS.md, there is no TypeScript package or skill surface in this diff. I did not check whether a changeset is expected for the Go module.

Generated by Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 861df3fc64

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…he client open One encrypt.Client serves every request in a service, but a call whose context was cancelled or passed its deadline closed it for all of them, and so did the auth profile a client reads its tokens from. Both runtimes are built with wazero's WithCloseOnContextDone, and every guest call ran under the caller's context, so the runtime closed the module when that context ended. Worse, the wait for the client's lock ignored the context: a request that timed out in the queue behind a slow ZeroKMS round trip reached the guest with a context already done, and wazero closed the module before any guest code ran. The fix keeps the caller's cancellation and takes it away from the runtime. internal/guest gains three pieces, used by encrypt.Client and auth.ProfileStore alike: - Lock, a one-slot lock whose wait ends with the caller's context and which refuses a context that has already ended, so a request that timed out in the queue never starts. auth.Strategy's lock uses it too. - Detach, the context a guest call runs under: the caller's values but not its cancellation, so the runtime no longer closes the module when a caller gives up. - Caller, which the host functions use to reach the caller's context: a ZeroKMS request, a CTS exchange or an OIDC provider call in flight is cancelled, fails as a transport failure, and the guest returns through its ordinary error path with its keys intact. The error matches both the context's error and ErrTransport. A backstop remains for a guest or host function that ignores cancellation: Detach's context ends guest.StuckAfter (30s) after the caller's, and the runtime then closes the module, as before. That close now means a bug, never a closed browser tab. TestInterruptedCallClosesTheClient pinned the old behaviour and is replaced by tests of the new contract: a call cancelled during a ZeroKMS request, during a token fetch, and during an auth strategy's exchange leaves the client and the profile open; a call whose deadline passes in the queue leaves without reaching the guest; a stuck call still closes the client and the profile; and many callers cancelling at random share one client without it closing. Closes #1169

…r its caller gives up, so a rotated refresh token is kept The token service rotates the refresh token when it answers a device session's refresh. A refresh cancelled with its caller after the service answered dropped the new token, and the next refresh sent one the service had retired, which it rejects: the strategy could not refresh again without a new login. This is the failure stack-auth's native refresh now avoids by finishing the refresh in a task after its caller leaves (#1172); the Go auth guest runs that refresh inline in WebAssembly, so the Go host has to do the same. Strategy.Token now runs its work on a goroutine that holds the strategy's lock until it is done, and returns to a caller whose context ends at once, without making a copy of a token nobody will read. The refresh call's context is marked, and only its requests run under guest.Detach of the caller's context with a grace of 10 seconds: the refresh finishes, the guest saves what came back, and the next Token reads it. Past the grace it fails as a transport failure, well inside the 30-second backstop, so the profile stays open. Close waits for such a refresh, so its rotated token is saved. Every other request to the token service, an access-key or OIDC exchange, rotates nothing and is still cancelled with its caller, as is a ZeroKMS request, so a cancelled NewClient or a shutdown does not wait on them. Refs #1169.

coderdan force-pushed the fix/go-client-cancel-isolation branch from 66ae295 to d431e3a Compare October 18, 2026 10:50

coderdan commented Oct 18, 2026

View reviewed changes

coderdan force-pushed the fix/go-client-cancel-isolation branch from d431e3a to a885a4d Compare October 18, 2026 11:31

This comment has been minimized.

Sign in to view

coderdan mentioned this pull request Oct 18, 2026

feat(golang)!: the guest steps instead of blocking on the network, and the client drives it outside its lock #1265

Merged

coderdan force-pushed the fix/go-client-cancel-isolation branch from a885a4d to 861df3f Compare October 19, 2026 01:00

coderdan marked this pull request as ready for review October 19, 2026 02:59

coderdan requested a review from a team as a code owner October 19, 2026 02:59

cipherstash-agent reviewed Oct 19, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread languages/golang/encrypt/client.go

coderdan added 2 commits October 19, 2026 16:19

coderdan force-pushed the fix/go-client-cancel-isolation branch from 861df3f to 94f995d Compare October 19, 2026 05:34

coderdan removed the request for review from a team October 19, 2026 06:02

coderdan merged commit b5ed21e into next Oct 19, 2026
30 checks passed

coderdan deleted the fix/go-client-cancel-isolation branch October 19, 2026 06:03

Navigation Menu

Repository navigation

fix(golang): a call whose context ends fails on its own, and leaves the client open - #1263

fix(golang): a call whose context ends fails on its own, and leaves the client open#1263
coderdan merged 2 commits into
nextfrom
fix/go-client-cancel-isolation

coderdan commented Oct 18, 2026 •

edited

Loading

Uh oh!

changeset-bot Bot commented Oct 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading

Review skipped

Uh oh!

coderdan commented Oct 18, 2026

Uh oh!

coderdan left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

coderdan commented Oct 19, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

cipherstash-agent left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

coderdan commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why it happened

Changes

Verification

Related

Review notes

Uh oh!

changeset-bot Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

coderabbitai Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

coderdan commented Oct 18, 2026

Uh oh!

coderdan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

coderdan commented Oct 19, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

cipherstash-agent left a comment

Choose a reason for hiding this comment

Review attestation: automated /code-review medium + /cross-check-review-findings

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderdan commented Oct 18, 2026 •

edited

Loading

changeset-bot Bot commented Oct 18, 2026 •

edited

Loading

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 19, 2026 •

edited

Loading

Review attestation: automated `/code-review medium` + `/cross-check-review-findings`